feat(starfish): send full block #6803
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 4 Skipped Deployments
|
VorobyevIlya
force-pushed
the
consensus/feat/starfish/send-full-block
branch
from
7a9fb27 to
c1e421a
Compare
VorobyevIlya
force-pushed
the
consensus/feat/starfish/send-full-block
branch
from
6ecd338 to
6c73112
Compare
piotrm50
force-pushed
the
consensus/feat/starfish-consensus
branch
from
6b470f7 to
3d6f76d
Compare
fix warnings fmt create add block headers command it compiles! fixing wrapper continues fix decoupling header from transactions decouple serialized bytes fix typo implement a wrapper around serialized header and transactions -- SerializedBlock (not finished) implement write and read functions for blocks for RocksDB fix test compilation fix compilation errors add a new struct VerifiedBlock
VorobyevIlya
force-pushed
the
consensus/feat/starfish/send-full-block
branch
from
b6ffaef to
be705ab
Compare
VorobyevIlya
force-pushed
the
consensus/feat/starfish/send-full-block
branch
from
be705ab to
7c46133
Compare
…_headers in mem_store
…lso saves the block into recent blocks
polinikita
reviewed
May 26, 2025
polinikita
reviewed
May 26, 2025
polinikita
reviewed
May 26, 2025
| // TODO: dedup block verifications, here and with fetched blocks. | ||
| let signed_block: SignedBlockHeader = | ||
| bcs::from_bytes(&serialized_block).map_err(ConsensusError::MalformedBlock)?; | ||
| let verified_block = VerifiedBlock::try_from(serialized_block)?; |
There was a problem hiding this comment.
The logic should be
Serialized bytes -> signed block header + transaction data -> verify signature -> check correctness of data commitment -> verified block
There was a problem hiding this comment.
Fixed it. Should we check the signature and commitment somewhere else?
polinikita
reviewed
May 26, 2025
polinikita
reviewed
May 26, 2025
| suspended_block_headers: BTreeMap<BlockRef, SuspendedBlockHeader>, | ||
| /// Keeps full blocks for suspended block headers | ||
| suspended_blocks: BTreeMap<BlockRef, VerifiedBlock>, | ||
| // TODO: should it be here? need to discuss |
There was a problem hiding this comment.
add a brief explanation. Is it ever ever-growing set? If so, it might be too big at the end.
There was a problem hiding this comment.
yes, there should be some eviction mechanism there. I will add to do.
polinikita
reviewed
May 26, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of change
This PR introduces a full block structure, which contains a header and transactions.
Changes in network communications:
In streaming we now use SerialiedBlocks structure which contain serialization of a header and serializetion of transactions.
In synchronizer Functions fetching blocks are removed or changed to fetch headers. The only place where we still fetch blocks is in the commitSyncer.
Commit:
Certified commit contains blocks, CommittedSubDag contains only headers for now, blocks should be added later. In stake verification we use only headers, since it is enough.
CoreThreadDispatcher: a new command AddBlockHeaders was added. Several Fake/Mock dispatchers were merged into one.
Storage: we store now both headers and full blocks. If blocks are written to disk then their headers are written there too. There is a possibility to read blocks or only headers. Changes implemented both for mem_store and RocksDB.
Linearizer now operates with headers.
DagState and BlockManager:
DagState works with headers now since for building DAG only headers are important. However, we still store recent_blocks since we need to request blocks for fetching.
Functions such as accept block work with headers now. However, there can be situations when we already have data but the corresponding block header is not accepted yet. We can also have accepted header without data. Structures for storing such blocks and headers were added to BlockManager. They should be cleaned at some point, it is something to be done in future PRs.
Links to any relevant issues
closes #6707.
Type of change
Choose a type of change, and delete any options that are not relevant.
How the change has been tested
Change checklist
Tick the boxes that are relevant to your changes, and delete any items that are not.
Release Notes